Add org uuid to the trace publishing token#756
Conversation
|
Warning Rate limit exceeded
To keep reviews running without waiting, you can enable usage-based add-on for your organization. This allows additional reviews beyond the hourly cap. Account admins can enable it under billing. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe changes extend JWT token generation with organization identity. The agent token controller and services now extract the Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
console/workspaces/libs/auth/src/no-auth/hooks/authHooks.ts (1)
22-31:⚠️ Potential issue | 🟡 MinorInconsistent
subclaim betweendemoUserInfoand JWT payload.The
demoUserInfoobject declaressub: 'default'(line 29), but the JWT token's payload contains"sub": "8f307351-25c5-4fc6-85e0-f51c2d458f06". Since the real Asgardio implementation spreadstokenInfo?.payloadinto the userInfo object, this mismatch could cause unpredictable behavior when code decodes the token versus usinguserInfodirectly.🔧 Align the JWT payload with demoUserInfo
Update the JWT payload to use
"sub": "default"to match line 29, or updatedemoUserInfo.subto match the JWT. For consistency, the stub token payload should mirror the stub userInfo.Also applies to: 42-42
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/libs/auth/src/no-auth/hooks/authHooks.ts` around lines 22 - 31, demoUserInfo.sub ('default') is inconsistent with the stub JWT payload's "sub" (GUID); update one to match the other so tokenInfo?.payload spread produces identical sub values—either change demoUserInfo.sub to "8f307351-25c5-4fc6-85e0-f51c2d458f06" or change the JWT payload's "sub" to "default" (also apply the same change to the other stub payload instance referenced around the second occurrence).agent-manager-service/middleware/jwtassertion/test_utils.go (1)
32-39:⚠️ Potential issue | 🔴 CriticalMock
OuIdvalue diverges from the test assertion.The mock seeds
OuId: "mock-org-id", butagent-manager-service/tests/agent_token_test.go(line 149) asserts the generated JWT claim equals"mock-ou-id". Whichever side is intended to be authoritative, these two literals must agree, otherwiseTestGenerateAgentToken/Generating a token for an external agent should return 200 with valid tokenwill fail. See the corresponding comment onagent_token_test.gofor the related JSON-tag mismatch (org_idvsou_id) that needs to be resolved at the same time.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent-manager-service/middleware/jwtassertion/test_utils.go` around lines 32 - 39, The mock TokenClaims in test_utils.go sets OuId to "mock-org-id" which conflicts with the test assertion expecting "mock-ou-id" in TestGenerateAgentToken; update the mock TokenClaims.OuId value to exactly match the expected literal ("mock-ou-id") or update the test assertion to match the mock, and at the same time reconcile the JSON tag mismatch between the struct field and test expectation (ensure the claim field name used by TokenClaims/RegisteredClaims serialization matches the test's expected key: "ou_id" vs "org_id"). Locate and fix the TokenClaims.OuId value and the JSON tag on the TokenClaims struct so the generated JWT claim and the test assertion are consistent.
🧹 Nitpick comments (2)
console/workspaces/libs/auth/src/no-auth/hooks/authHooks.ts (1)
42-42: JWT payload missing expected user claims.The JWT payload only contains standard claims (
iss,iat,exp,aud,sub) and the newouIdfield, but is missing several claims present indemoUserInfo(lines 22-31):username,displayName,orgHandle,orgId,orgName,sessionState, andallowedScopes.Since the real Asgardio implementation spreads the decoded JWT payload into the UserInfo object, the stub token should contain equivalent claims to ensure consistent behavior when code decodes and uses the token versus directly accessing
userInfo.💡 Populate JWT payload with demoUserInfo fields
Consider regenerating the JWT with a payload that includes all relevant fields from
demoUserInfo:{ "iss": "Agent Management Platform Local", "iat": 1761727469, "exp": 1793263469, "aud": "localhost", "sub": "default", "username": "john.doe", "displayName": "John Doe", "orgHandle": "default", "orgId": "default", "orgName": "Default", "ouId": "faa183f1-3983-4f63-bb35-86fa723fd5ef" }This ensures that decoding the stub token yields the same user information as the
demoUserInfoobject.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@console/workspaces/libs/auth/src/no-auth/hooks/authHooks.ts` at line 42, The JWT returned by getToken() is missing user claims present in demoUserInfo (username, displayName, orgHandle, orgId, orgName, sessionState, allowedScopes), causing mismatch when code decodes the token vs using userInfo; regenerate or replace the hardcoded token in getToken to a JWT whose payload includes those demoUserInfo fields (matching names: username, displayName, orgHandle, orgId, orgName, sessionState, allowedScopes, plus existing ouId/iss/aud/sub/iat/exp) so decoded token and demoUserInfo are equivalent; update the token string in getToken to the new JWT.agent-manager-service/services/agent_token_manager.go (1)
244-300: Consider validatingreq.OrgIdat the service boundary.Both current call sites (
controllers/agent_token_controller.goandservices/agent_manager.go) validate thatOrgIdis non-empty before reaching here, so this is defensive only. However, the service is a public API and a future caller could passreq.OrgId == "", which would silently produce a token with"org_id": "". A short guard would localize the invariant and prevent that drift.🛡️ Proposed defensive check
func (s *agentTokenManagerService) GenerateToken(ctx context.Context, req GenerateTokenRequest) (*spec.TokenResponse, error) { s.logger.Info("Generating token for agent", "agentName", req.AgentName, "orgName", req.OrgName, "projectName", req.ProjectName, ) + + if req.OrgId == "" { + return nil, fmt.Errorf("OrgId is required: %w", utils.ErrInvalidInput) + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@agent-manager-service/services/agent_token_manager.go` around lines 244 - 300, Add a defensive guard at the start of agentTokenManagerService.GenerateToken to reject empty OrgId: check if req.OrgId == "" (before calling s.ocClient.GetComponent) and if so log an error and return an invalid-input error (e.g., combine utils.ErrInvalidInput with a descriptive error like "org id is required") so the service never issues a token with an empty OrgId.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@agent-manager-service/services/agent_manager.go`:
- Around line 433-438: The code in generateAgentAPIKey checks
jwtassertion.GetTokenClaims(ctx) but when callerClaims is nil or
callerClaims.OuId == "" it logs and returns "", translatePipelineError(err)
which uses a nil err and thus returns nil; change this to return a meaningful
sentinel error (e.g., utils.ErrMissingOrgIdentity or utils.ErrForbidden) instead
of translatePipelineError, so callers get a non-nil error and upstream can map
to 403; update the return in the failing branch to return "",
utils.ErrMissingOrgIdentity (or your project’s equivalent) and keep the existing
log statement to preserve diagnostics.
In `@agent-manager-service/tests/agent_token_test.go`:
- Around line 141-149: The test is asserting the wrong claim key and value;
update the assertions in agent_token_test.go to match the implementation: check
for "org_id" (not "ou_id") and expect the value "mock-org-id" for the org claim
— this aligns with AgentTokenClaims.OrgId (json:"org_id") in
agent-manager-service/services/agent_token_manager.go and the mock setting
TokenClaims.OuId = "mock-org-id" in
agent-manager-service/middleware/jwtassertion/test_utils.go; change
require.Contains(t, claims, "ou_id") → require.Contains(t, claims, "org_id") and
require.Equal(t, "mock-ou-id", claims["ou_id"]) → require.Equal(t,
"mock-org-id", claims["org_id"]).
---
Outside diff comments:
In `@agent-manager-service/middleware/jwtassertion/test_utils.go`:
- Around line 32-39: The mock TokenClaims in test_utils.go sets OuId to
"mock-org-id" which conflicts with the test assertion expecting "mock-ou-id" in
TestGenerateAgentToken; update the mock TokenClaims.OuId value to exactly match
the expected literal ("mock-ou-id") or update the test assertion to match the
mock, and at the same time reconcile the JSON tag mismatch between the struct
field and test expectation (ensure the claim field name used by
TokenClaims/RegisteredClaims serialization matches the test's expected key:
"ou_id" vs "org_id"). Locate and fix the TokenClaims.OuId value and the JSON tag
on the TokenClaims struct so the generated JWT claim and the test assertion are
consistent.
In `@console/workspaces/libs/auth/src/no-auth/hooks/authHooks.ts`:
- Around line 22-31: demoUserInfo.sub ('default') is inconsistent with the stub
JWT payload's "sub" (GUID); update one to match the other so tokenInfo?.payload
spread produces identical sub values—either change demoUserInfo.sub to
"8f307351-25c5-4fc6-85e0-f51c2d458f06" or change the JWT payload's "sub" to
"default" (also apply the same change to the other stub payload instance
referenced around the second occurrence).
---
Nitpick comments:
In `@agent-manager-service/services/agent_token_manager.go`:
- Around line 244-300: Add a defensive guard at the start of
agentTokenManagerService.GenerateToken to reject empty OrgId: check if req.OrgId
== "" (before calling s.ocClient.GetComponent) and if so log an error and return
an invalid-input error (e.g., combine utils.ErrInvalidInput with a descriptive
error like "org id is required") so the service never issues a token with an
empty OrgId.
In `@console/workspaces/libs/auth/src/no-auth/hooks/authHooks.ts`:
- Line 42: The JWT returned by getToken() is missing user claims present in
demoUserInfo (username, displayName, orgHandle, orgId, orgName, sessionState,
allowedScopes), causing mismatch when code decodes the token vs using userInfo;
regenerate or replace the hardcoded token in getToken to a JWT whose payload
includes those demoUserInfo fields (matching names: username, displayName,
orgHandle, orgId, orgName, sessionState, allowedScopes, plus existing
ouId/iss/aud/sub/iat/exp) so decoded token and demoUserInfo are equivalent;
update the token string in getToken to the new JWT.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 61275ac8-a18d-4faa-9e2b-26c95307e541
📒 Files selected for processing (6)
agent-manager-service/controllers/agent_token_controller.goagent-manager-service/middleware/jwtassertion/test_utils.goagent-manager-service/services/agent_manager.goagent-manager-service/services/agent_token_manager.goagent-manager-service/tests/agent_token_test.goconsole/workspaces/libs/auth/src/no-auth/hooks/authHooks.ts
2 participants
Purpose
Goals
Approach
User stories
Release note
Documentation
Training
Certification
Marketing
Automation tests
Security checks
Samples
Related PRs
Migrations (if applicable)
Test environment
Learning
Summary by CodeRabbit
New Features
Tests